My opinion is that it would be cleaner to move this to the derived IoSocketHandle- what does the fd() method resolve to for interfaces that are not sockets? Either that, or there can be a generic method to manage a descriptor of sorts, such that for sockets, the descriptor is really just the fd. And for other interfaces it might be a session id, etc.

I would prefer a generic method to return some sorts of identity which can be used differently for different implementation. WDYT?

@danzh1989 @sbelair2 I don't think I'm grokking this convo fully. Can one or both of you expand? I don't think we should still need this method so I'm likely missing something.

@mattklein123 Maybe I missed some earlier conversation, but what shall be passed into evconnlistener_new() if there is no fd() or something similar in interface?

We could probably figure out an interface strategy in which the socket actually creates the listener somehow, but for now, I would probably just do a dynamic_cast. It's a little ugly, but VPP or similar will have to have their own listener implementation, so it should fit together OK? Thoughts?

@mattklein123 Do you mean to make fd() a method to IoSocketHandle only and dynamic_cast the IoHhandle object when we create listener? That sounds reasonable to me.

Yes that's what I meant. IMO that's OK for now.

It seems to me that there is probably no point in defining the template class IoHandleCallResult if all the virtual methods in the base class are all IoHandleCallResult or IoHandleCallResult<ssize_t>. I think we need to examine if there are any return values that can be other than integer types. If there are other interfaces that need this (I'm not saying that VPP is one of them), then won't these virtual methods preclude them?

@danzh2010 I do want to say that I like the direction where this is going.

Are you concerned about some implementations of same method whose return a type is different from what is declared here? For such method, I would say same purpose can be achieved by adding another new method to the interface.

If IoHandleCallResult and IoHandleCallResult<ssize_t> are the only two return types we need, what would you like the IoHandleCallResult to be without template?

@danzh2010 Sorry, forgot about this. I'm only thinking that we should not preclude further options, but maybe that's just too much caution? Right now I cannot think of other return types we might need, but that doesn't mean that such a need won't come up later.

Now we have IoHandleCallResult<std::bitset<2>>

nit: how will getIoHandleName() and getPeerName() actually return the name? They take const params, whereas getsockname() takes a mutable out-param. (Though making the Network::Address::Instance param non-const may not be sufficient, since Address::Instance isn't mutable in a way that allows it to be changed to a different address.)

yeah we will need some type of out param for these functions. You could either do an out param is an actual parameter, or have it return a struct of a result code and the address. I actually prefer the latter but I don't feel strongly about it.

I would prefer an output param of Network::Address::Instance type.

For UDP we will need methods that provide the equivalent of sendmmsg and recvmmsg.

Added. Not sure if envoy has something equivalent to mmsghdr or msghdr.

I'm not aware of any equivalent currently--the closest I can think of would be the UdpData struct @conqerAtapple added recently. Though if we wanted to use that, we would have to add analogues for msghdr.msg_control and msghdr.msg_flags fields. @conqerAtapple , do you have an opinion?

I would suggest we use an enum for the options we care about. The one gotcha here is IIRC we have some capability in the config to actually set options that the code doesn't know about, by int, so maybe this isn't possible and we need to keep this? Thoughts?

Would there be potential crash if envoy fails to map the value set in config and the enum defined here? If so, introducing enum for socket option can be error-prone.

Right I think if we have a config option to set an int, we need to leave it as is.

enum for options we care about?

Added enum IoHandleFlag, but we only used O_NONBLOCK so far.

Return a bit set of enums that we care about?

do you mean std::bitset or just int?

I'm not aware of any equivalent currently--the closest I can think of would be the UdpData struct @conqerAtapple added recently. Though if we wanted to use that, we would have to add analogues for msghdr.msg_control and msghdr.msg_flags fields. @conqerAtapple , do you have an opinion?

nit: prefer include header vs. forward decl unless there is a circular dependency, same below.

include/envoy/buffer/buffer.h will depend on io_handle.h, so will network/address.h.

nit: add comments for what each of these mean generically to Envoy?

done

nit: Why explicitly number this?

...and ditto for the enums above (ShutdownType, IOError, ...).

nit: Why explicitly number this?

This enum's values should be mapped to 2^n if we want it to fit into bitset.

OK, can you use 0x1, 0x10, etc. do make that clear?

I assume you meant 0b1, 0b10. Done

Oops sorry I meant 0x1, 0x2, 0x4, 0x10, etc. but either way.

Can we document what the bitset actually means? Potentially with an enum of each bit?

done

I think you can just refer back to the enum which will have 0x1, etc. for docs?

done

We could probably figure out an interface strategy in which the socket actually creates the listener somehow, but for now, I would probably just do a dynamic_cast. It's a little ugly, but VPP or similar will have to have their own listener implementation, so it should fit together OK? Thoughts?

if this merges after #5772, can we also update that API to reference IOError here rather than do what it's doing?

Also IMO IOError should not be underneath the Network namespace, as this also makes sense for non-networked I/O.

This PR won't be checked in as a whole. Otherwise, all the implementations of these interfaces will have to be there and replacing all the direct sys call's to fd. I plan to check in the methods one by one in following PR's, starting from #5171 which will very likely has IoErrorCode definition.

Where would you like to place IoError?
And it will be great if #5772 can have TODO for switching to IoError if it's checked in first.